Skip to content

Conversation

@yiyuan-he
Copy link
Contributor

@yiyuan-he yiyuan-he commented May 1, 2025

What does this pull request do?

Adds data handling logic for Gen AI spans with Large Language Objects (LLOs). LLO attribute/value in SpanProvider pipeline will be dropped and emitted as Log Events to the LogsProvider pipeline.

The dependency requirements of our ADOT SDK have been loosened to reduce dependency collision with wide variety of instrumentation libraries and frameworks.

TODO:

  • Finalize Log Events schema to align with OTel spec.
  • Unit tests

Test plan

Instrumented different sample apps with various combinations of instrumentation set ups with this dev version of ADOT SDK and sent data to OTLP X-Ray and Log endpoints.

Genesis PoC

TBA:

  • Langchain + Traceloop PoC
  • Langchain + OpenInference PoC
  • Langchain + OpenLit PoC
  • Langchain + Openllmetry
  • CrewAI + Traceloop
  • CrewAI + OpenInference
  • CrewAI + OpenLit
  • CrewAI + Openllmetry

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@yiyuan-he yiyuan-he requested a review from a team as a code owner May 1, 2025 21:56
@yiyuan-he yiyuan-he requested a review from mxiamxia May 1, 2025 21:57
Comment on lines 89 to 100
metadata = {
"trace_id": format(span.context.trace_id, 'x'),
"span_id": format(span.context.span_id, 'x'),
"attribute_name": key,
"span_name": span.name
}

# Get S3 pointer from LLOSenderClient
s3_pointer = self._llo_sender_client.upload(value, metadata)

# Store the S3 pointer instead of original value to trim span
updated_attributes[key] = s3_pointer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should we do if LLO handling fails?



def _customize_logs_exporter(log_exporter: LogExporter, resource: Resource) -> LogExporter:
return OTLPAwsLogExporter(endpoint=os.getenv(OTEL_EXPORTER_OTLP_LOGS_ENDPOINT))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what will be the default logs_endpoint if it is not specified by Env var?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will align with the default from the SigV4 Logs Exporter PR. I plan to drop my PoC implementation and plug that in once it is merged.

Comment on lines +15 to +16
AWS_CLOUDWATCH_LOG_GROUP_ENV = "AWS_CLOUDWATCH_LOG_GROUP"
AWS_CLOUDWATCH_LOG_STREAM_ENV = "AWS_CLOUDWATCH_LOG_STREAM"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prashant shared with me their changes for logs exporter, I think we should use the existing env var OTEL_EXPORTER_OTLP_LOGS_HEADERS=x-aws-log-group=[CW-LOG-GROUP-NAME],x-aws-log-stream=[CW-LOG-STREAM-NAME] defined by OTel

#358

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are unused vars I need to clean up. Currently, the our Logs Pipelines work out of the box with OTEL_EXPORTER_OTLP_LOGS_HEADERS since I am running the application with:

λ  env OTEL_METRICS_EXPORTER=none \
       OTEL_TRACES_EXPORTER=otlp \
       OTEL_LOGS_EXPORTER=otlp \
       OTEL_PYTHON_DISTRO=aws_distro \
       OTEL_PYTHON_CONFIGURATOR=aws_configurator \
       OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf \
       OTEL_EXPORTER_OTLP_LOGS_HEADERS="x-aws-log-group=test,x-aws-log-stream=default" \
       OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=https://xray.us-east-1.amazonaws.com/v1/traces \
       OTEL_EXPORTER_OTLP_LOGS_ENDPOINT=https://logs.us-east-1.amazonaws.com/v1/logs \
       OTEL_RESOURCE_ATTRIBUTES="service.name=langchain-app" \
       OTEL_PYTHON_LOGGING_AUTO_INSTRUMENTATION_ENABLED="true" \
       OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT="true" \
       OTEL_PYTHON_DISABLED_INSTRUMENTATIONS="http,sqlalchemy,psycopg2,pymysql,sqlite3,aiopg,asyncpg,mysql_connector,botocore,boto3,urllib3,requests,starlette" \
       python run_customer_support_console.py

self._aws_region = None
self._has_required_dependencies = False
# set up logger for LLO data
self._logs_exporter = OTLPAwsLogExporter(endpoint=os.getenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can self._logs_exporter be initiated outside of class and pass in by callers per need?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am working on some changes to factor out the LLO-related logic into a separate LLOHandler class that may address this concern. Will follow-up on this to see if it is sufficient after I push those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with a dependency injection approach to initialize self._logs_exporter outside of class and pass to LLOHandler utility class.

OTLPAwsLogsExporter initialized in aws_opentelemetry_configurator.py -> OTLPAwsSpanExporter -> LLOHandler

Comment on lines 53 to 54
self._logger_provider = LoggerProvider()
self._logger_provider.add_log_record_processor(BatchLogRecordProcessor(self._logs_exporter))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same idea, it looks like we can just get the initialized logger instance pass in for Span exporter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be addressed in other thread

@yiyuan-he yiyuan-he force-pushed the handle-llo-span-exporter branch from f9e6af2 to 99ef646 Compare May 7, 2025 17:47
@yiyuan-he yiyuan-he force-pushed the handle-llo-span-exporter branch from 99ef646 to 5f30abf Compare May 7, 2025 22:25
@yiyuan-he yiyuan-he force-pushed the handle-llo-span-exporter branch from 5f30abf to 15832d0 Compare May 7, 2025 22:37
@yiyuan-he
Copy link
Contributor Author

Closing in favor of #361

@yiyuan-he yiyuan-he closed this May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants